Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Windows Compatibility(for lib/TH) #2439

Merged
merged 7 commits into from Sep 17, 2017

Conversation

peterjc123
Copy link
Collaborator

Win64 support for lib/TH

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!
I did a first pass on it, and had a look on the output of the contribuild. TH compiles, but the compiler outputs some warnings that might be good to fix before merging this PR.
Also, I didn't check the implementation inside THAllocator.c.

{
#if defined(USE_C11_ATOMICS)
return atomic_fetch_add(a, value);
#elif defined(USE_MSC_ATOMICS)
assert(sizeof(int) == sizeof(long));
return _InterlockedExchangeAdd((long*)a, value);
//assert(sizeof(int32_t) == sizeof(long));

This comment was marked as off-topic.

{
#if defined(USE_C11_ATOMICS)
return atomic_compare_exchange_strong(a, &oldvalue, newvalue);
#elif defined(USE_MSC_ATOMICS)
assert(sizeof(int) == sizeof(long));
return (_InterlockedCompareExchange((long*)a, (long)newvalue, (long)oldvalue) == (long)oldvalue);
//assert(sizeof(int32_t) == sizeof(long));

This comment was marked as off-topic.

IMPLEMENT_THFILE_STORAGE(Char, int8_t)
IMPLEMENT_THFILE_STORAGE(Short, int16_t)
IMPLEMENT_THFILE_STORAGE(Int, int32_t)
IMPLEMENT_THFILE_STORAGE(Long, int32_t)

This comment was marked as off-topic.

TH_API void THBlas_(copy)(long n, real *x, long incx, real *y, long incy);
TH_API void THBlas_(axpy)(long n, real a, real *x, long incx, real *y, long incy);
TH_API real THBlas_(dot)(long n, real *x, long incx, real *y, long incy);
TH_API void THBlas_(swap)(int64_t n, real *x, int64_t incx, real *y, int64_t incy);

This comment was marked as off-topic.

@@ -922,7 +922,7 @@ THDescBuff THTensor_(desc)(const THTensor *tensor) {
int i;
for(i = 0; i < tensor->nDimension; i++) {
if(n >= L) break;
n += snprintf(str+n, L-n, "%ld", tensor->size[i]);
n += snprintf(str+n, L-n, "%lld", tensor->size[i]);

This comment was marked as off-topic.

_q = THTensor_fastGet1d(q, rand_ind);

_mask = THRandom_bernoulli(_generator, _q);
_mask = (int) THRandom_bernoulli(_generator, _q);

This comment was marked as off-topic.

@@ -556,7 +556,7 @@ static size_t THDiskFile_readString(THFile *self, const char *format, char **str
total += TBRS_BSZ;
p = THRealloc(p, total);
}
if (fgets(p+pos, total-pos, dfself->handle) == NULL) /* eof? */
if (fgets(p+pos, (int) (total-pos), dfself->handle) == NULL) /* eof? */
{
if(pos == 0)

This comment was marked as off-topic.

@fmassa
Copy link
Member

fmassa commented Aug 17, 2017

Thanks for the changes!
TH seems now to compile without any warnings.
The build fails because of the dependency of THPP in TH, so we should probably only merge these PRs together.
@apaszke any better ideas on how we can run the tests with all the changes? The monolithic PR was too hard to review, the separate PRs are easier but running the full test suite is trickier.

@peterjc123
Copy link
Collaborator Author

peterjc123 commented Aug 17, 2017

@fmassa I tried to make CI builds with windows-full branch. Every time a change is made, it will be merged into this branch. Maybe it's useful.
For Linux/Mac Build Status
For Windows Build status

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your effort!
I think this is ready for merge. TH builds without warnings.
Let's wait until the other folders are reviewed so that we can merge them all.

#elif defined(TH_REAL_IS_LONG)
TH_TENSOR_APPLY(real, self, *self_data = (long)(THRandom_random(_generator) % (LONG_MAX+1UL)););
TH_TENSOR_APPLY(real, self, *self_data = (int64_t)THRandom_random(_generator););

This comment was marked as off-topic.

@soumith soumith added this to Ready for Review in PR Status Aug 23, 2017
@soumith soumith merged commit 61813cf into pytorch:master Sep 17, 2017
@soumith
Copy link
Member

soumith commented Sep 17, 2017

Thanks a ton @peterjc123 for doing this big PR. This is really good stuff.

@soumith soumith removed this from Ready for Review in PR Status Sep 25, 2017
@peterjc123 peterjc123 deleted the windows-TH branch September 27, 2017 02:15
houseroad added a commit to houseroad/pytorch that referenced this pull request Dec 10, 2019
…c39b3b (pytorch#30619)

Summary:
Pull Request resolved: pytorch#30619

Previous import was fea8568cac61a482ed208748fdc0e1a8e47f62f5

Included changes:
- **[c08a7b76](onnx/onnx@c08a7b76)**: doc: fix some typos at ONNXIFI (pytorch#2473) <Yorkie Liu>
- **[4be12d46](onnx/onnx@4be12d46)**: remove workshop update since it is done (pytorch#2460) <Prasanth Pulavarthi>
- **[86107d1b](onnx/onnx@86107d1b)**: Updated with correct URL to LICENSE (pytorch#2468) <Ryan Loney>
- **[9bf6fbb6](onnx/onnx@9bf6fbb6)**: Update Argmin/Argmax (pytorch#2461) <Lara Haidar>
- **[748d81b8](onnx/onnx@748d81b8)**: Fix windows conda build (pytorch#2452) <Ashwini Khade>
- **[a32db1c5](onnx/onnx@a32db1c5)**: Delete duplicate word in comment (pytorch#2439) <Haibo Hao>
- **[e108da9a](onnx/onnx@e108da9a)**: Fix bug in function body verifier (pytorch#2390) <G. Ramalingam>
- **[c3d3ef82](onnx/onnx@c3d3ef82)**: docs: fix typo in IR.md (pytorch#2441) <Elliot Waite>

Test Plan: ci

Reviewed By: hl475

Differential Revision: D18766132

fbshipit-source-id: f5767698c83ee7f0055f4cb87db9fb434c0dc9df
facebook-github-bot pushed a commit that referenced this pull request Dec 10, 2019
…c39b3b (#30619)

Summary:
Pull Request resolved: #30619

Previous import was fea8568cac61a482ed208748fdc0e1a8e47f62f5

Included changes:
- **[c08a7b76](onnx/onnx@c08a7b76)**: doc: fix some typos at ONNXIFI (#2473) <Yorkie Liu>
- **[4be12d46](onnx/onnx@4be12d46)**: remove workshop update since it is done (#2460) <Prasanth Pulavarthi>
- **[86107d1b](onnx/onnx@86107d1b)**: Updated with correct URL to LICENSE (#2468) <Ryan Loney>
- **[9bf6fbb6](onnx/onnx@9bf6fbb6)**: Update Argmin/Argmax (#2461) <Lara Haidar>
- **[748d81b8](onnx/onnx@748d81b8)**: Fix windows conda build (#2452) <Ashwini Khade>
- **[a32db1c5](onnx/onnx@a32db1c5)**: Delete duplicate word in comment (#2439) <Haibo Hao>
- **[e108da9a](onnx/onnx@e108da9a)**: Fix bug in function body verifier (#2390) <G. Ramalingam>
- **[c3d3ef82](onnx/onnx@c3d3ef82)**: docs: fix typo in IR.md (#2441) <Elliot Waite>

Test Plan: ci

Reviewed By: hl475

Differential Revision: D18766132

fbshipit-source-id: 13c04f21399579acb87a8f9fac2e4c329b0720b8
wuhuikx pushed a commit to wuhuikx/pytorch that referenced this pull request Jan 30, 2020
…c39b3b (pytorch#30619)

Summary:
Pull Request resolved: pytorch#30619

Previous import was fea8568cac61a482ed208748fdc0e1a8e47f62f5

Included changes:
- **[c08a7b76](onnx/onnx@c08a7b76)**: doc: fix some typos at ONNXIFI (pytorch#2473) <Yorkie Liu>
- **[4be12d46](onnx/onnx@4be12d46)**: remove workshop update since it is done (pytorch#2460) <Prasanth Pulavarthi>
- **[86107d1b](onnx/onnx@86107d1b)**: Updated with correct URL to LICENSE (pytorch#2468) <Ryan Loney>
- **[9bf6fbb6](onnx/onnx@9bf6fbb6)**: Update Argmin/Argmax (pytorch#2461) <Lara Haidar>
- **[748d81b8](onnx/onnx@748d81b8)**: Fix windows conda build (pytorch#2452) <Ashwini Khade>
- **[a32db1c5](onnx/onnx@a32db1c5)**: Delete duplicate word in comment (pytorch#2439) <Haibo Hao>
- **[e108da9a](onnx/onnx@e108da9a)**: Fix bug in function body verifier (pytorch#2390) <G. Ramalingam>
- **[c3d3ef82](onnx/onnx@c3d3ef82)**: docs: fix typo in IR.md (pytorch#2441) <Elliot Waite>

Test Plan: ci

Reviewed By: hl475

Differential Revision: D18766132

fbshipit-source-id: 13c04f21399579acb87a8f9fac2e4c329b0720b8
samnordmann pushed a commit to samnordmann/pytorch that referenced this pull request Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants